Add a grammar health checker for two unenforced preconditions - #475
Open
johnml1135 wants to merge 1 commit into
Open
Add a grammar health checker for two unenforced preconditions#475johnml1135 wants to merge 1 commit into
johnml1135 wants to merge 1 commit into
Conversation
HermitCrab imposes two requirements on a grammar that nothing reports today, so a grammar author only learns of a violation as a parse that silently returns nothing. Every segment used must be declared in a CharacterDefinitionTable: an undeclared segment makes the parser refuse every word containing it. And each segment needs a distinct phonological feature bundle within its table: when two share one, the parser cannot reliably determine which morphemes are involved. GrammarHealthChecker.Check(Language) reports both as findings with a severity, a stable code and the offending declarations named. It is diagnostic only -- a grammar the engine would load still loads, and nothing throws. Lives in the netstandard2.0 engine library rather than a tool, so FieldWorks and any other host can call it directly on a loaded Language. The duplicate-bundle check is skipped for a grammar that declares no PhonologicalFeatureSystem at all. Such a grammar distinguishes segments by their representation alone, so every bundle is the same empty struct by construction and reporting it would be a false positive on a correct grammar.
johnml1135
force-pushed
the
feature/grammar-health-checker
branch
from
August 20, 2026 09:32
b12553f to
0e80833
Compare
johnml1135
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #475 +/- ##
==========================================
- Coverage 73.53% 73.48% -0.05%
==========================================
Files 449 451 +2
Lines 37633 37830 +197
Branches 5174 5202 +28
==========================================
+ Hits 27673 27799 +126
- Misses 8824 8888 +64
- Partials 1136 1143 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
johnml1135
added a commit
that referenced
this pull request
Aug 20, 2026
GrammarHealthChecker and GrammarHealthFinding are a diagnostic feature, not part of the conformance suite: nothing in conformance/ or in the Conformance project calls them, and their only caller here was their own test file. They are wanted in FieldWorks on a schedule of their own, so they ship in #475 instead, which now carries the newer copies this branch had developed. Two tests go with them that cannot follow them there. Both load real conformance fixtures, and one uses the Conformance project's Fixture.DiscoverAll, so neither compiles on a branch that has no fixture tree. They belong here rather than in #475, and should come back once that lands; until then they are recoverable from 9dfb2f6. What remains of this branch's engine footprint is Trace.FailureAllomorph, its assignment in TraceManager from an argument that method already received and discarded, and the inert SemanticBranch capture point.
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HermitCrab imposes two requirements on a grammar that nothing currently reports. A grammar author only discovers a violation as a parse that silently returns nothing, which is a hard failure to diagnose from the outside.
Every segment used must be declared in a
CharacterDefinitionTable. An undeclared segment makes the parser refuse every word containing it — total, and silent-looking.Each segment needs a distinct phonological feature bundle within its table. When two segments share one, the parser cannot reliably determine which morphemes are involved.
GrammarHealthChecker.Check(Language)reports both as findings carrying a severity, a stable code, and the offending declarations named so a host can navigate to them.Design notes
Diagnostic only. It never throws and never changes parse behaviour — a grammar the engine would load still loads. It reports; the host decides what to do.
In the
netstandard2.0engine library, not a tool. So it ships in theSIL.Machine.Morphology.HermitCrabNuGet package and any host — FieldWorks, or anyone testing their own grammar — can call it directly on a loadedLanguage. Verified by packing:lib/netstandard2.0/SIL.Machine.Morphology.HermitCrab.dll.The duplicate-bundle check is skipped when a grammar declares no
PhonologicalFeatureSystemat all. Such a grammar distinguishes segments by their representation alone, so every bundle is the same empty struct by construction. Reporting that would be a false positive on a correct grammar, and grammars of exactly this shape exist.Tests
Five tests covering both checks, the clean case, and the feature-less-grammar case. They build the object model directly rather than loading XML, so they have no external fixture dependency. Full HermitCrab suite passes (98/98), zero-warning build, CSharpier and
gitlintclean.Provenance
Rebased onto current
master(it was based on 3.9.2). The three files here are also present on the HermitCrab conformance branch (#480), where they were developed further; this branch now carries that newer version, and #480 drops them so the checker lands on its own schedule. Nothing in the conformance suite calls the checker, so the split is clean. Two additional tests live on that branch which run the checker across the real conformance fixtures — they need the fixture tree and the Conformance project, so they stay there rather than move here.🤖 Generated with Claude Code
This change is